Skip to content

fix(stubgen): clean read-only staged files after failure - #4974

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
ideabsassistenza:fix/stubgen-readonly-cleanup
Oct 14, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
ideabsassistenza:fix/stubgen-readonly-cleanup

Conversation

@ideabsassistenza

@ideabsassistenza ideabsassistenza commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

On Windows, a staged stub-generator file inherits a read-only destination's mode. If a later output cannot be staged or replacement is denied, cleanup cannot unlink that temporary file. It leaves the file behind and its PermissionError masks the original failure.

When temporary-file removal is denied, make that staged file writable and retry removal. The existing destination keeps its contents and read-only mode. Generated code, output permissions and the successful write path remain unchanged.

Tested

Windows, CPython 3.12.14, based on main at a373241de4403f7e8272be09ccf1a6b0866a8c69; tested commit 149aea506152466d0f0befa14c747105ab29f810:

  • All 13 stub-generator tests pass, including two new regressions using actual Windows read-only files.
  • Both new tests fail with the unmodified current upstream tools/stubgen.py and pass with the fix. One verifies the original missing-directory diagnostic and temporary cleanup; the other verifies cleanup after Windows denies replacing a read-only destination. Both preserve the original file and its mode.
  • The full Python tools suite passes: 248 tests.
  • Repository conventions and git diff --check pass.
  • Reviewed the open stubgen-related work: fix(stubgen): write LF to stdout on Windows #3919 changes stdout newlines and test(tools): create hard links with os.link for Python 3.9 #4545 replaces a hard-link test API for Python 3.9. Neither changes this cleanup or implements the fix; their changes touch different lines.

No native runtime code changed. No full native build, CTest or title run was performed. Tests were run directly with Python; the Windows-specific replacement test is skipped on other platforms.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file; no native exports or silent stubs changed
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source; none added
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • AI-assisted: yes (Codex)

@ideabsassistenza
ideabsassistenza force-pushed the fix/stubgen-readonly-cleanup branch from 4d4f6a9 to 149aea5 Compare October 13, 2026 07:48
@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test: head 149aea506 merged onto current main cac4e9688 (clean merge, no conflicts). Windows 10, MinGW GCC (WinLibs), Ninja Release, -j2: full build 0 errors; ctest with the filter ^stubgen$: stubgen passed (1 test(s), 0 failed); python tools/check_conventions.py --base upstream/main passed. I did not run the new tests against main without the change.

AI-assisted (Claude Code).

@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the cleanup change in write_outputs and the two new regressions: build and scoped ctest pass on Linux, the Windows-only test is skipped there as designed, and @xXPBSXx's Windows run covers it. Merging once the Windows build is green.

@adriadam10
adriadam10 merged commit c2b55c6 into boykopovar:main Oct 14, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants